Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

move backend to GPUArraysCore.jl #420

Merged
merged 5 commits into from
Aug 17, 2022
Merged

move backend to GPUArraysCore.jl #420

merged 5 commits into from
Aug 17, 2022

Conversation

N5N3
Copy link
Contributor

@N5N3 N5N3 commented Jul 26, 2022 •
edited
Loading

Second take of JuliaGPU/Adapt.jl#50.
This PR wants to move backend to GPUArraysCore.jl, which would help us to implement broadcast for StructArray on gpu. (see JuliaArrays/StructArrays.jl#215)
I hope it's also useful for other arraywrappers.
(I only bump GPUArraysCore's version as we haven't registered a new GPUArrays.)

@N5N3 N5N3 mentioned this pull request Jul 27, 2022
Merged
5 tasks
@maleadt
Copy link
Member

maleadt commented Aug 11, 2022 •
edited
Loading

I'm not sure about this, backend wasn't really intended as a public API. Would something like JuliaGPU/Adapt.jl#52 work as well, using functionality from Adapt.jl (which I assume is lightweight enough)? On the other hand, if you don't need the recursive behavior (to figure out the back-end of a nested GPUArray) maybe GPUArrays.backend is sufficient.

maleadt
maleadt requested changes Aug 12, 2022
View reviewed changes
Copy link
Member

@maleadt maleadt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I'd just change the export/import, but this is good to go otherwise.

lib/GPUArraysCore/src/GPUArraysCore.jl Show resolved Hide resolved
src/device/execution.jl Show resolved Hide resolved
@N5N3
Copy link
Contributor Author

N5N3 commented Aug 12, 2022 •
edited
Loading

@maleadt I'm assuming we don't want to export backend from GPUArrays.jl?
We have @reexport using GPUArraysCore, so exporting GPUArraysCore.backend would also export backend from GPUArrays.
I think we'd better revert [0137d18] as otherwise there seems no advantage to use Reexport. (@reexport dont support something like @reexport using A except backend,)

@maleadt
Copy link
Member

maleadt commented Aug 12, 2022

@maleadt I'm assuming we don't want to export backend from GPUArrays.jl?
We have @reexport using GPUArraysCore, so exporting GPUArraysCore.backend would also export backend from GPUArrays.

Oh, no, we don't want that indeed, good catch.

@maleadt maleadt merged commit 26c5191 into JuliaGPU:master Aug 17, 2022
@maleadt
Copy link
Member

maleadt commented Aug 21, 2022

Why did you change the JLArrays compat here? Adding a ~ doesn't relax but tighten the requirements, right?

@N5N3 N5N3 deleted the move_backend branch August 22, 2022 00:41
@N5N3
Copy link
Contributor Author

N5N3 commented Aug 22, 2022

Oh yes. I must misread the Pkg document.

@maleadt maleadt mentioned this pull request Sep 20, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maleadt maleadt maleadt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@N5N3 @maleadt